Verdict: comment — close to mergeable, but a couple of important issues worth addressing first.

The design is right. Resolving auto to a concrete "software" | "hardware" once per process, failing safe to software on any probe error, mirroring the real render args in the probe, and keeping the engine-config default at "software" while flipping CLI to "auto" — all the right calls. Tests cover pass-through, probe failure, caching, and the CLI tri-state. Two real issues, one stale-doc miss, and a few nits.

Important

• packages/engine/src/services/browserManager.ts, resolveBrowserGpuMode — concurrent-probe race. parallelCoordinator.executeWorkerTask fans out N workers via Promise.all in the same process (see parallelCoordinator.ts:259 — tasks.map(...) then Promise.all). Each worker calls createCaptureSession → resolveBrowserGpuMode. While _autoBrowserGpuModeCache === undefined, every worker hits the cache !== undefined guard, sees undefined, and launches its own probe Chrome. On a no-GPU host running --workers 4, that's 4 simultaneous Chrome launches fighting for CPU/RAM at startup — exactly the hosts where Chrome launch is slowest and most fragile. The PR body's "~1-2 s on first render" assumes one probe; under contention the slowest of 4 concurrent launches is materially worse. Fix is small: cache the promise, not the result —

  let _autoBrowserGpuModeProbe: Promise<"software" | "hardware"> | undefined;

  First call assigns, subsequent calls await the same promise. The PR's Notes section says "If a worker pool spawns multiple workers each launches one probe — acceptable since the probe is cheap." That reasoning only holds for separate-process worker pools; the engine's parallelCoordinator is in-process, so the assumption doesn't apply here. Worth fixing before this lands.

• packages/cli/src/docs/rendering.md (lines 23, 29) — docs are now wrong. Line 23: "enabled by default for local renders". Line 29: "Local renders use browser GPU capture automatically". After this PR the local default is auto (probe-then-decide), not "browser GPU on." Both lines need to mention the new behaviour and the auto/hardware/software env value. Stale-by-omission docs are a real footgun on a flag that flips Chrome's rendering backend.

• packages/engine/src/services/browserManager.ts, resolveBrowserGpuMode — no observability on which path was picked. The PR body itself flags "misclassification toward software is the safe failure mode" — which means production will silently pick software when hardware would have worked, and we'll have no log line to diagnose it. One console.info("[BrowserManager] auto browser GPU resolved to '<mode>' (probed in Xms)") (or gated on config.debug) is enough to answer "is the auto-probe actually firing on $host?" from logs. As-is, the only signal is "renders feel slow." Add this before merge.

Nits

• packages/engine/src/services/browserManager.ts:339 — getBrowserGpuArgs parameter type. With "auto" now in EngineConfig["browserGpuMode"], the function silently accepts a value the caller is supposed to have already collapsed. The defensive if (mode === "auto") branch (lines around 433–437 of the new code) papers over a type-system gap. Cleaner: tighten the param to mode: "software" | "hardware" and require callers to pass a resolved mode. The "should not reach here" becomes a compile-time guarantee instead of a runtime fallback.

• packages/engine/src/services/browserManager.ts — _autoBrowserGpuModeCache exported as let. export let _autoBrowserGpuModeCache plus _resetAutoBrowserGpuModeCacheForTests is a fine test-escape-hatch pattern, but the underscore is convention-only — anything in the workspace can import { _autoBrowserGpuModeCache } and read it. Either keep it module-private and only export the reset (tests don't actually need to inspect the cache), or move both behind a __test__ namespace that's lint-flagged for non-test use.

• packages/engine/src/services/browserManager.ts — probe could be tighter than getContext("webgl") !== null. On some Linux VMs --use-gl=egl returns a context that's actually backed by software (Mesa llvmpipe / SwiftShader-via-ANGLE). The probe says "hardware" but the real render is no faster than software — and we paid a Chrome launch for nothing. A read of gl.getParameter(gl.UNMASKED_RENDERER_WEBGL) (via the WEBGL_debug_renderer_info extension) and rejecting /SwiftShader|llvmpipe|Software/i would tighten this. Edge case, optional.

• packages/engine/src/services/browserManager.ts — --ignore-gpu-blocklist in the probe. The probe sets it; the real render args (via buildChromeArgs) also set it (line 286). Consistent — but worth confirming explicitly that any future change to buildChromeArgs keeps the probe and render args in sync. A single getBaseChromeArgs() helper shared by both call sites would prevent the two from drifting.

Praise

• The fail-safe-to-software policy on any probe error (Chrome launch, missing canvas API, navigation, etc.) is exactly right. Misclassifying toward software is recoverable (slow render); misclassifying toward hardware is a render-time failure. The PR commits to the correct default direction.
• Engine-config default stays "software", CLI default flips to "auto". That correctly draws the line between embedders (parity-harness, producer's eval pipeline) who don't want surprise probe-on-launch, and CLI users who want it Just Work. Good separation of concerns.
• --enable-unsafe-swiftshader rationale in the comment block in getBrowserGpuArgs is the right amount of "future engineer reading this won't have to re-derive why we set a flag named unsafe". Keep that level of comment everywhere.
• PRODUCER_BROWSER_GPU_MODE=auto is now first-class in both the engine config (config.ts:177) and the CLI resolver, with symmetric handling. Nice.

— Vai

Solid feature — auto-detection is the right call for making CPU-only hosts Just Work. The probe → cache → fallback architecture is clean. A few items to address (echoing + extending Vai's review):

1. Concurrent probe race (Important)
_autoBrowserGpuModeCache stores the resolved string, not the Promise. With --workers 4 on a CPU-only host, all 4 workers call resolveBrowserGpuMode("auto") near-simultaneously. All see undefined cache, all launch Chrome probes. Wasteful (4x the ~2s probe cost) and nondeterministic timing.

Fix: cache the Promise<"software" | "hardware"> itself:

let _autoBrowserGpuModePromise: Promise<"software" | "hardware"> | undefined;

export async function resolveBrowserGpuMode(mode, options) {
  if (mode !== "auto") return mode;
  if (!_autoBrowserGpuModePromise) {
    _autoBrowserGpuModePromise = probeWebGLAvailability(options);
  }
  return _autoBrowserGpuModePromise;
}

First caller creates the promise, concurrent callers await the same one. Zero extra launches.

2. Stale docs (Important)
packages/cli/src/docs/rendering.md lines 23 and 29 still say "enabled by default for local renders" — should say "auto-detected" to match the new behavior. Same PR, easy fix.

3. Log the resolved mode (Important)
Add a console.log (or whatever the logger is) when auto-mode resolves. Without it, "always falls back to software even with GPU present" would be invisible in production logs. One line:

console.log(`[engine] auto GPU probe: ${_autoBrowserGpuModeCache}`);

4. getBrowserGpuArgs("auto") defensive fallback (Nit)
The defensive "auto" case in getBrowserGpuArgs is good, but worth a console.warn since reaching it means the resolution step was skipped — a logic bug, not a runtime condition.

Everything else looks correct: fail-safe-to-software on any probe error, correct embedder/CLI split (engine defaults software, CLI defaults auto), --enable-unsafe-swiftshader comment is clear, test coverage maps onto the new behavior.

All three important items cleanly addressed in 076b6144:

1. Concurrent probe race — cache is now Promise<"software" | "hardware">, not the resolved string. The p1 === p2 === p3 reference equality test proves dedup at the API level. Correct.
2. Stale docs — rendering.md lines 23 + 29 updated to describe the auto/hardware/software trichotomy.
3. Log line — [hyperframes] browserGpuMode auto → <mode> (<reason>) on stderr, once per process. Reason includes "WebGL probe succeeded" / "WebGL unavailable" / "probe failed (...)" / "puppeteer unavailable". Cache hits don't re-log. Good.

The IIFE pattern for the cached promise (_autoBrowserGpuModeCache = (async () => { ... })()) is clean — captures the options from the first caller and deduplicates all concurrent awaits.

536/536 engine + 256/256 CLI. LGTM.

Re-verdict: Approve.

All 3 important items from my prior review cleanly resolved in 076b6144:

• #1 Concurrent-probe race — fixed at the root. Cache type is now Promise<"software" | "hardware"> | undefined so the first caller's in-flight Promise is what subsequent concurrent callers await. The new regression test asserts reference equality across simultaneous probe calls (p1 === p2 === p3) — that's the exact contract pinned at the API level, not just "same eventual result". Single Chrome launch on --workers 4 on a no-GPU host now, instead of N.
• #2 Stale rendering.md — lines 23 + 29 updated to describe the auto / hardware / software trichotomy explicitly. User-visible contract now matches the code.
• #3 Resolved-mode log line — [hyperframes] browserGpuMode auto → <mode> (<reason>) emitted once per process when the probe resolves. Reason field is specific (WebGL probe succeeded / WebGL unavailable / probe failed (...) / puppeteer unavailable), so a regression to silent-software-fallback would be diagnosable from production logs immediately. Cache hits correctly don't re-log.

Verification claims (engine 536/536, CLI 256/256, lint/format/typecheck clean) match the test-file expansion in the diff. Skipping the 4 nits per the team's "cheap-only" preference is fine.

Three rounds, three clean responses. Shipping.

— Vai